autostart: Ensure instance is started/stopped by `launchd` or `systemctl` #4139

norio-nomura · 2025-10-05T08:51:15Z

This PR is formed with two parts:

autostart: Refactor to add autoStartManager to prepare for the following commit
autostart: Ensure the instance is started/stopped by launchd or systemctl

Refactor to add `autoStartManager`

Hide creating/deleting an autostart entry file into Register/Unregister as the implementation details.
Separate the launchd and systemd-specific code into packages.

Use `launchctl` or `systemctl` to start/stop the instance if it’s registered to autostart.

If the instance isn’t launched by launchd, it won’t be stopped on log out.

Affected sub commands:

limactl start
limactl stop
limactl restart
limactl edit
limactl shell

API changes

`pkg/autostart`

Introduced AutoStartedIdentifier():
- If not empty, it indicates whether the instance was started by launchd or systemd.
Added RequestStart():
- Delegates the operation to launchd or systemd.
Added RequestStop():
- Delegates the operation to launchd or systemd.

`pkg/autostart/launchd`

Added AutoStartedServiceName():
- Uses the XPC_SERVICE_NAME environment variable as the service name.
Added RequestStart():
- Uses launchctl enable service-target to avoid failing bootstrap.
- Uses launchctl bootstrap domain-target plist-path.
Added RequestStop():
- Uses launchctl bootout service-target if the instance is launched by launchd.
Added --progress to the limactl option in io.lima-vm.autostart.INSTANCE.plist:
- Required to support limactl start --progress.

`pkg/autostart/systemd`

Added AutoStartedServiceName():
- Uses CurrentUnitName() by github.com/coreos/go-systemd/v22/util as the service identifier.
Added RequestStart():
- Uses systemctl --user start unit-name.
Added RequestStop():
- Uses systemctl --user stop unit-name if the instance is launched by systemd.
Added --progress to the limactl option in [email protected]:
- Required to support limactl start --progress.

`pkg/hostagent`

Add AutoStartedIdentifier to Info.
- If not empty, it indicates whether the instance was started by launchd or systemd.

`pkg/instance`

StartWithPaths():
- Use autostart.IsRegistered() to check if the instance is registered to autostart.
- If launchHostAgentForeground is true, ignore autostart registration.
- If the instance is registered to autostart, use autostart.RequestStart() instead of launching HostAgent.
StopGracefully():
- Use autostart.RequestStop().
Restart(), RestartForcibly():
- Use autostart.IsRegistered() to skip networks.Reconcile() if the instance is registered to autostart.

AkihiroSuda · 2025-10-06T00:29:19Z

How to test this PR?

AkihiroSuda · 2025-10-06T00:30:04Z

Also please explain what issue is being solved by this PR

norio-nomura · 2025-10-06T00:59:52Z

Also please explain what issue is being solved by this PR

opened #4141

go.mod

AkihiroSuda · 2025-10-09T07:32:09Z

pkg/autostart/systemd/systemd.go

Should be *_linux.go?

Code that is platform-dependent and can only be built on Linux is already isolated in systemd_linux.go. systemd.go has code that can be built on any platform, only the operation is platform-dependent.

If we put the code itself included in systemd.go into systemd_linux.go, I think the number of dummy codes required for systemd_others.go will increase.

pkg/autostart/systemd/[email protected]

AkihiroSuda · 2025-10-09T07:33:08Z

pkg/autostart/launchd/launchd.go

Should be *_darwin.go

The code included in launchd.go is based on the darwin platform, but the build is not based on the darwin platform.

The runtime guard should be enough.

pkg/autostart/launchd/io.lima-vm.autostart.INSTANCE.plist

norio-nomura · 2025-10-09T08:57:48Z

How to test this PR?

I was researching how to test and took a detour to fix test-port-forwarding.pl. I will consider it again.

pkg/autostart/launchd/launchd_test.go

pkg/autostart/systemd/systemd_test.go

jandubois

Sorry, didn't have time to do a proper review yet, so this is more like first-impressions from browsing the code.

I also feel like the OS-specific code could be separated better into files that compile only on the respective platforms. Especially the manager should not have references to Launchd or Systemd in it, but work generically via an interface.

cmd/limactl/delete.go

jandubois · 2025-10-31T04:53:33Z

cmd/limactl/edit.go

+	// Network reconciliation will be performed by the process launched by the autostart manager
+	if registered, err := autostart.IsRegistered(ctx, inst); err != nil && !errors.Is(err, autostart.ErrNotSupported) {
+		return fmt.Errorf("failed to check if the autostart entry for instance %q is registered: %w", inst.Name, err)
+	} else if !registered {
+		err = reconcile.Reconcile(ctx, inst.Name)
+		if err != nil {
+			return err
+		}


This logic seems to be repeated in several places. Should it be part of reconcile.Reconcile() instead?

There are several reconcile.Reconcile() calls that are not related to autostart.IsRegistered().
It is not good to put it in reconcile.Reconcile().

jandubois · 2025-10-31T04:55:02Z

cmd/limactl/start-at-login_unix.go

-			logrus.Infof("The autostart file %q has been created or updated", autostart.GetFilePath(runtime.GOOS, inst.Name))
+	if registered, err := autostart.IsRegistered(ctx, inst); err != nil {
+		return fmt.Errorf("failed to check if the autostart entry for instance %q is registered: %w", inst.Name, err)
+	} else if startAtLogin {


Don't use else when the if branch ends in a return. @alexandear I'm surprised we don't have a linter rule enabled for this.

Suggested change

} else if startAtLogin {

}

if startAtLogin {

The linter rule should be subject to return in the last else block.

pkg/autostart/launchd/io.lima-vm.autostart.INSTANCE.plist

jandubois · 2025-10-31T05:14:48Z

pkg/autostart/managers.go

+var Launchd = &TemplateFileBasedManager{
+	filePath:              launchd.GetPlistPath,
+	template:              launchd.Template,
+	enabler:               launchd.EnableDisableService,
+	autoStartedIdentifier: launchd.AutoStartedServiceName,
+	requestStart:          launchd.RequestStart,
+	requestStop:           launchd.RequestStop,
+}
+
+// Systemd is the autostart manager for Linux.
+var Systemd = &TemplateFileBasedManager{
+	filePath:              systemd.GetUnitPath,
+	template:              systemd.Template,
+	enabler:               systemd.EnableDisableUnit,
+	autoStartedIdentifier: systemd.AutoStartedUnitName,
+	requestStart:          systemd.RequestStart,
+	requestStop:           systemd.RequestStop,
+}


This feels weird. Why is TemplateFileBasedManager not a regular Go interface?

TemplateFileBasedManager is one of the implementations of the type autoStartManager interface.
The differences between platforms are divided into pkg/autostart/launchd and pkg/autostart/systemd.
TemplateFileBasedManager has a common part that is not platform-dependent.

jandubois · 2025-10-31T05:16:38Z

pkg/instance/restart.go

-		return err
+	if registered, err := autostart.IsRegistered(ctx, inst); err != nil && !errors.Is(err, autostart.ErrNotSupported) {
+		return fmt.Errorf("failed to check if the autostart entry for instance %q is registered: %w", inst.Name, err)
+	} else if !registered {


Suggested change

} else if !registered {

}

if !registered {

To do that, it is necessary to expand the scope of the registered and err variables.

If it is a variable that is necessary outside of if... {...} else if ... {...}, I will expand the scope, but if the scope of use is only this place, I don't want to expand the scope.

norio-nomura · 2025-10-31T08:10:24Z

I also feel like the OS-specific code could be separated better into files that compile only on the respective platforms. Especially the manager should not have references to Launchd or Systemd in it, but work generically via an interface.

The parts where the build depends on the platform have already been separated.

Although it is platform-dependent at the time of execution, the logic itself is platform-independent, so unit testing is currently possible across platforms.

If it is changed so that it is built only on the corresponding platform, defects that can be found in unit tests on macOS will only be found on CI's Linux.

- Hide creating/deleting an autostart entry file into `Register`/`Unregister` as the implementation details. - Separate the `launchd` and `systemd`-specific code into packages. Signed-off-by: Norio Nomura <[email protected]>

…ctl` If the instance isn’t launched by `launchd`, it won’t be stopped on log out. This changes to use `launchctl` or `systemctl` to start/stop the instance if it’s registered to autostart. ## Affected sub commands: - `limactl start` - `limactl stop` - `limactl restart` - `limactl edit` - `limactl shell` ## API changes ### `pkg/autostart` - Introduced `AutoStartedIdentifier()`: - If not empty, it indicates whether the instance was started by `launchd` or `systemd`. - Added `RequestStart()`: - Delegates the operation to `launchd` or `systemd`. - Added `RequestStop()`: - Delegates the operation to `launchd` or `systemd`. ### `pkg/autostart/launchd` - Added `AutoStartedServiceName()`: - Uses the XPC_SERVICE_NAME environment variable as the service name. - Added `RequestStart()`: - Uses `launchctl enable service-target` to avoid failing `bootstrap`. - Uses `launchctl bootstrap domain-target plist-path`. - Added `RequestStop()`: - Uses `launchctl bootout service-target` if the instance is launched by `launchd`. - Added `--progress` to the `limactl` option in `io.lima-vm.autostart.INSTANCE.plist`: - Required to support `limactl start --progress`. ### `pkg/autostart/systemd` - Added `AutoStartedServiceName()`: - Uses `CurrentUnitName()` by `github.com/coreos/go-systemd/v22/util` as the service identifier. - Added `RequestStart()`: - Uses `systemctl --user start unit-name`. - Added `RequestStop()`: - Uses `systemctl --user stop unit-name` if the instance is launched by `systemd`. - Added `--progress` to the `limactl` option in `[email protected]`: - Required to support `limactl start --progress`. ### `pkg/hostagent` - Add `AutoStartedIdentifier` to `Info`. - If not empty, it indicates whether the instance was started by `launchd` or `systemd`. ### `pkg/instance` - `StartWithPaths()`: - Use `autostart.IsRegistered()` to check if the instance is registered to autostart. - If `launchHostAgentForeground` is true, ignore autostart registration. - If the instance is registered to autostart, use `autostart.RequestStart()` instead of launching HostAgent. - `StopGracefully()`: - Use `autostart.RequestStop()`. - `Restart()`, `RestartForcibly()`: - Use `autostart.IsRegistered()` to skip `networks.Reconcile()` if the instance is registered to autostart. Signed-off-by: Norio Nomura <[email protected]> bump `coreos/go-systemd` to v22.6.0 Signed-off-by: Norio Nomura <[email protected]> # Conflicts: # cmd/limactl/edit.go # cmd/limactl/shell.go # cmd/limactl/start.go # pkg/instance/restart.go

…hctl bootstrap` Because instances may be stopped without unloading the plist file. e.g. `limactl stop -f` or `limactl factory-reset`. If the plist file is not unloaded, `launchctl bootstrap` will fail. Signed-off-by: Norio Nomura <[email protected]>

Signed-off-by: Norio Nomura <[email protected]>

- Remove wrong INFO message - Remove the `—progress` option. Signed-off-by: Norio Nomura <[email protected]>

Signed-off-by: Norio Nomura <[email protected]>

norio-nomura · 2025-11-02T00:51:36Z

I also feel like the OS-specific code could be separated better into files that compile only on the respective platforms.

Separated OS-specific code.

AkihiroSuda

Thanks

norio-nomura · 2025-11-02T09:11:35Z

Thanks! 🙏🏻

norio-nomura marked this pull request as draft October 5, 2025 08:55

norio-nomura force-pushed the improve-autostart branch 7 times, most recently from 783c0f2 to 428fcb1 Compare October 5, 2025 12:13

norio-nomura marked this pull request as ready for review October 5, 2025 12:42

AkihiroSuda linked an issue Oct 6, 2025 that may be closed by this pull request

limactl should use launchctl to start/stop instance if it's registered to start-at-login #4141

Closed

norio-nomura force-pushed the improve-autostart branch 2 times, most recently from ac9db28 to 31d7007 Compare October 6, 2025 07:05

AkihiroSuda added this to the v2.0.0 milestone Oct 6, 2025

norio-nomura force-pushed the improve-autostart branch 4 times, most recently from 89c500c to 7082f48 Compare October 8, 2025 15:42

AkihiroSuda reviewed Oct 9, 2025

View reviewed changes

go.mod Outdated Show resolved Hide resolved

AkihiroSuda reviewed Oct 9, 2025

View reviewed changes

pkg/autostart/systemd/[email protected] Outdated Show resolved Hide resolved

AkihiroSuda reviewed Oct 9, 2025

View reviewed changes

pkg/autostart/launchd/io.lima-vm.autostart.INSTANCE.plist Outdated Show resolved Hide resolved

norio-nomura force-pushed the improve-autostart branch from 7082f48 to 994efee Compare October 9, 2025 08:18

norio-nomura force-pushed the improve-autostart branch from 994efee to f3861a0 Compare October 12, 2025 08:47

AkihiroSuda requested a review from a team October 14, 2025 02:33

alexandear reviewed Oct 14, 2025

View reviewed changes

pkg/autostart/launchd/launchd_test.go Outdated Show resolved Hide resolved

pkg/autostart/systemd/systemd_test.go Outdated Show resolved Hide resolved

norio-nomura force-pushed the improve-autostart branch 3 times, most recently from 2af6c71 to 497f9ed Compare October 21, 2025 00:08

AkihiroSuda requested a review from a team October 21, 2025 14:47

norio-nomura force-pushed the improve-autostart branch from 497f9ed to 4a9a91e Compare October 24, 2025 03:55

AkihiroSuda requested a review from a team October 27, 2025 16:09

norio-nomura force-pushed the improve-autostart branch from 4a9a91e to 89ec621 Compare October 30, 2025 08:48

jandubois reviewed Oct 31, 2025

View reviewed changes

norio-nomura force-pushed the improve-autostart branch from d9a7159 to 53ab3e5 Compare October 31, 2025 08:01

norio-nomura added 5 commits November 2, 2025 09:12

autostart: Refactor to add autoStartManager

d9a122b

- Hide creating/deleting an autostart entry file into `Register`/`Unregister` as the implementation details. - Separate the `launchd` and `systemd`-specific code into packages. Signed-off-by: Norio Nomura <[email protected]>

Use //go:build !windows to exclude tests

0e11d6e

Signed-off-by: Norio Nomura <[email protected]>

Apply reviews

3eb1197

- Remove wrong INFO message - Remove the `—progress` option. Signed-off-by: Norio Nomura <[email protected]>

norio-nomura force-pushed the improve-autostart branch from 53ab3e5 to 90514e7 Compare November 2, 2025 00:33

pkg/autostart: Avoid importing code that is not used during runtime.

1613e09

Signed-off-by: Norio Nomura <[email protected]>

norio-nomura force-pushed the improve-autostart branch from 90514e7 to 1613e09 Compare November 2, 2025 00:48

AkihiroSuda approved these changes Nov 2, 2025

View reviewed changes

AkihiroSuda merged commit 4a47790 into lima-vm:master Nov 2, 2025
37 checks passed

norio-nomura deleted the improve-autostart branch November 2, 2025 09:11

autostart: Ensure instance is started/stopped by launchd or systemctl #4139

autostart: Ensure instance is started/stopped by launchd or systemctl #4139

Uh oh!

Conversation

norio-nomura commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactor to add autoStartManager

Use launchctl or systemctl to start/stop the instance if it’s registered to autostart.

Affected sub commands:

API changes

pkg/autostart

pkg/autostart/launchd

pkg/autostart/systemd

pkg/hostagent

pkg/instance

Uh oh!

AkihiroSuda commented Oct 6, 2025

Uh oh!

AkihiroSuda commented Oct 6, 2025

Uh oh!

norio-nomura commented Oct 6, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

norio-nomura commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

norio-nomura commented Oct 31, 2025

Uh oh!

norio-nomura commented Nov 2, 2025

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

norio-nomura commented Nov 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

autostart: Ensure instance is started/stopped by `launchd` or `systemctl` #4139

autostart: Ensure instance is started/stopped by `launchd` or `systemctl` #4139

norio-nomura commented Oct 5, 2025 •

edited

Loading

Refactor to add `autoStartManager`

Use `launchctl` or `systemctl` to start/stop the instance if it’s registered to autostart.

`pkg/autostart`

`pkg/autostart/launchd`

`pkg/autostart/systemd`

`pkg/hostagent`

`pkg/instance`